Skip to content

Fix "No Detail" Use Cases for Legacy and Generational ZGC logging #448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix "No Detail" Use Cases for Legacy and Generational ZGC logging #448

wants to merge 3 commits into from

Conversation

jlittle-ptc
Copy link
Contributor

Re-did the changes from PR #440 after a conflicting PR was brought in first.

Specifically, this PR addresses the following use cases:

Log lines from a Java 17/21 Legacy ZGC log without details (no gc*)

[0.038s][info][gc] Using The Z Garbage Collector
[1.810s][info][gc] GC(0) Garbage Collection (Metadata GC Threshold) 122M(2%)->66M(1%)
[2.429s][info][gc] GC(1) Garbage Collection (Metadata GC Threshold) 150M(2%)->96M(1%)
[4.339s][info][gc] GC(2) Garbage Collection (Metadata GC Threshold) 348M(4%)->72M(1%)
[26.051s][info][gc] GC(3) Garbage Collection (Warmup) 816M(10%)->128M(2%)

Log lines from Java 21 (Generational) GC Log without details:

[4.252s][info][gc] GC(2) Major Collection (Metadata GC Threshold)
[4.366s][info][gc] GC(2) Major Collection (Metadata GC Threshold) 384M(5%)->186M(2%) 0.114s
[36.326s][info][gc] GC(3) Major Collection (Warmup)
[36.423s][info][gc] GC(3) Major Collection (Warmup) 730M(9%)->258M(3%) 0.097s

@jlittle-ptc jlittle-ptc mentioned this pull request Jun 23, 2025
Closed
@jlittle-ptc
Copy link
Contributor Author

Just checking that this will be merged at some point? I really don't want to have to redo them a second time if another breaking change makes it in first :)

@dsgrieve
Copy link
Contributor

@d3r3kk @karianna

karianna
karianna approved these changes Jul 14, 2025
View reviewed changes
parser/src/main/java/com/microsoft/gctoolkit/parser/ZGCParser.java

// We use the lack of a MemorySummary in the forwardReference as a sign that this is the case and
// that we need to publish a Generational no-details event.
ZGCCycleType type = ZGCCycleType.get(trace.getGroup(2));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whitespace is slightly out

parser/src/main/java/com/microsoft/gctoolkit/parser/ZGCParser.java
getForwardRefForPhase(ZGCPhase.MAJOR_YOUNG) :
getForwardRefForPhase(ZGCPhase.MINOR_YOUNG);

if (forwardReference != null && !forwardReference.hasMemorySummary()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpcik, but I have a slight preference for putting that logic into a function that effectively says what you said in the comment. e.g. Not the hill I'm going to die on though :-)

// Comment
if (noMemorySummary()) {
....

@karianna
Copy link
Member

@jlittle-ptc Apologies, PR LGTM, just some small nitpicks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karianna karianna karianna approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jlittle-ptc @dsgrieve @karianna